Skip to content

Comments

SF-3727 Display format options if a build was completed#3707

Open
pmachapman wants to merge 1 commit intomasterfrom
fix/SF-3727
Open

SF-3727 Display format options if a build was completed#3707
pmachapman wants to merge 1 commit intomasterfrom
fix/SF-3727

Conversation

@pmachapman
Copy link
Collaborator

@pmachapman pmachapman commented Feb 23, 2026

This PR fixes a bug on live when after a draft was completed, and before formatting options were selected, a new build was started then cancelled. This PR allows access to the format options screen in those cases (but does not allow previewing formatting)


Open with Devin

This change is Reviewable

@pmachapman pmachapman added the will require testing PR should not be merged until testers confirm testing is complete label Feb 23, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@codecov
Copy link

codecov bot commented Feb 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.74%. Comparing base (db4bbd7) to head (3d58f6a).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3707   +/-   ##
=======================================
  Coverage   81.73%   81.74%           
=======================================
  Files         619      619           
  Lines       38651    38655    +4     
  Branches     6317     6319    +2     
=======================================
+ Hits        31593    31598    +5     
- Misses       6084     6096   +12     
+ Partials      974      961   -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Nateowami
Copy link
Collaborator

Nateowami commented Feb 23, 2026

I don't understand this change very well. It sounds like at this point in the process it's too late to select the formatting options, so we ...let the user select them anyway, and then do nothing with those options? That seems confusing and counter-productive. It seems like it would make more sense to just use the defaults and not enforce the requirement to select formatting options.

Or maybe I'm completely misunderstanding the problem and this change. My first though on reading the issue was that the behavior described sounds like what I would expect.

@pmachapman
Copy link
Collaborator Author

Or maybe I'm completely misunderstanding the problem and this change. My first though on reading the issue was that the behavior described sounds like what I would expect.

@Nateowami The problem we had was that a user generated a draft, then when that draft completed, they generated a new one, but realising their error, cancelled it.

Because they did not select formatting options for that first draft, they were unable to use the draft, except download it as USFM (they wanted to use the apply-per-chapter feature of the editor's draft tab).

The only real way they could have gained access to this function was to generate a whole new draft, then configure formatting options.

This PR is designed for this unusual edge case. I was scratching my head as to other ways we could have handled this, so if you have other ideas, I am all ears.

Copy link
Collaborator

@RaymondLuong3 RaymondLuong3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the edge case that is being described. In my opinion, this is user error and we should just tell the user to generate a new draft and format the draft. I can see that this may be a pain point to the projects that might run into this, but I really do not expect enough projects to experience this that it is worth supporting this workflow.

@RaymondLuong3 reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: 2 of 5 files reviewed, all discussions resolved.

@pmachapman
Copy link
Collaborator Author

@Nateowami @RaymondLuong3 OK, so shall we close this PR and issue as wont fix?

@RaymondLuong3
Copy link
Collaborator

That is what I would be in favour of, but I'll leave it to @Nateowami for the final decision

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

will require testing PR should not be merged until testers confirm testing is complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants